Set maxOutputLength correctly#16604
Conversation
|
@arhimondr now clue how to test it though for now. The |
| sink.getSlice().byteArray(), | ||
| sink.getSlice().byteArrayOffset() + bytesPreserved, | ||
| sink.getSlice().length()); | ||
| sink.getSlice().length() - bytesPreserved); |
There was a problem hiding this comment.
Actually code below:
System.arraycopy(
source.getSlice().byteArray(),
source.getSlice().byteArrayOffset() + source.getPosition(),
sink.getSlice().byteArray(),
sink.getSlice().byteArrayOffset() + bytesPreserved,
blockSize);
seems broken too. Do we know that blockSize will always fit in the targetSlice given some of the data is taken by bytesPreserved @arhimondr ?
There was a problem hiding this comment.
int bufferSize = blockSizeInBytes
// to guarantee a single long can always be read entirely
+ Long.BYTES;
buffers[0] = new ReadBuffer(Slices.allocate(bufferSize))
The buffer size is allocated according to blockSize with an extra 8 bytes (one long) for carry-overs.
The carry over is guaranteed to never be higher than 8 bytes as this is the most what can be requested to be read in one shot.
losipiuk
force-pushed
the
lo/fix-lzo-buffer
branch
from
e671f64 to
d0898ef
Compare
|
@arhimondr I tried to add regression test (attempt is here) but with no luck. I cannot trigger the code flow which results in check failure. |
Previously we were not accounting for bytesPreserved. With recent check added to aircompressor code (airlift/aircompressor@127b7f3) it started manifesting as java.lang.IllegalArgumentException. This is strictly validation fix. The passed array was not filled beyond allocated size.
losipiuk
force-pushed
the
lo/fix-lzo-buffer
branch
from
d0898ef to
f50081c
Compare
|
@arhimondr updated with test |
2 participants
Previously we were not accounting for bytesPreserved.
With recent check added to aircompressor code
(airlift/aircompressor@127b7f3)
it started manifesting as java.lang.IllegalArgumentException.
This is strictly validation fix. The passed array was not filled beyond
allocated size.
Description
fixes: #16541
Additional context and related issues
Release notes
( ) This is not user-visible or docs only and no release notes are required.
(x) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: